GLOWS - fix empty issue#3180
Conversation
tech3371
left a comment
tech3371
left a comment
There was a problem hiding this comment.
This update looks good besides one minor comment
| hist_l1a_list = valid_hists | ||
|
|
||
| # Filter out histograms with no recorded events (all-zero histogram data). | ||
| valid_hists = [hist for hist in hist_l1a_list if hist.number_of_events > 0] |
There was a problem hiding this comment.
I did same in the past for GLOWS and feel like it didn't catch all cases of invalid histogram. I hope checking hist.number_of_events > 0 is enough to filter out valid histogram. @maxinelasp may be better help here.
There was a problem hiding this comment.
Pull request overview
This PR improves GLOWS L1A histogram CDF generation robustness by excluding histogram records that are effectively “empty” due to having zero recorded events, and adds a unit test to validate the filtering behavior.
Changes:
- Added filtering in
generate_histogram_datasetto drop histograms wherenumber_of_events == 0and emit a warning when exclusions occur. - Added a unit test to ensure mixed histogram inputs do not include zero-event records in the output dataset.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
imap_processing/glows/l1a/glows_l1a.py |
Adds an additional validity filter for L1A histogram inputs (number_of_events > 0) with warning logging. |
imap_processing/tests/glows/test_glows_l1a_cdf.py |
Adds a unit test covering filtering behavior for mixed valid and zero-event histogram inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| f"histogram(s) with number_of_events == 0." | ||
| ) | ||
| hist_l1a_list = valid_hists | ||
|
|
|
|
||
| assert len(dataset["epoch"].values) == 2 | ||
|
|
||
|
|
This pull request improves the robustness of the
generate_histogram_datasetfunction by filtering out invalid histogram data and adds a corresponding unit test to ensure this behavior. The main focus is on ignoring histograms with zero recorded events, which prevents them from affecting downstream data processing.Data filtering improvements:
generate_histogram_datasetinglows_l1a.pyto filter outHistogramL1Aobjects wherenumber_of_events == 0, and added a warning log when such histograms are excluded.Testing enhancements:
test_generate_histogram_dataset_filters_zero_eventsintest_glows_l1a_cdf.pyto verify that histograms with zero events are filtered out and do not appear in the resulting dataset.